-
Notifications
You must be signed in to change notification settings - Fork 80
use POSIX feature macro to check for clock_gettime #348
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
note: while _POSIX_TIMERS is "directly" defined by including time.h on some systems, it may only be done if the internal macros are defined, which is commonly done by including unistd.h, so that is done before tested to work on Debian and MSYS2 (gcc + clang) fixes #345 (C part)
|
ping @Bill-Gray for review/merge |
|
I just tried building WinGUI and WinCon, cross-compiling on my Linux box. For both, I got : So... compiled without trouble, but couldn't link. Presumably, there's something a little more subtle about when the various timing functions are available to us... any ideas? I also built with Digital Mars and OpenWATCOM 1.9, each for DOS and WinCon, and it all worked and the resulting I also compiled everything with both of those compilers for OS/2. I mention this just for completeness, since I don't actually have an OS/2-capable machine on which to test the results. I got build errors with DMC, but I've had those before (don't know why); it's not related to the current issue. With OpenWATCOM, everything compiled without warnings or errors. So I think if we can figure out a fix for the above error message, we'd be good. |
|
For clock_gettime we'd nee to add -lrt (for glibc only necessary before 2.17, see https://www.man7.org/linux/man-pages/man3/clock_gettime.3.html - but likely no problem to do so in any case), no? Can you please test if adding it to the Makefile(s) works for your other build environments? |
|
Sorry, should have just given this a go right away... especially since the only change required on my machine was adding Unfortunately, it didn't work, because (with my MinGW version 9.3-win32 20200320) /* only newer MinGW environments have clock_gettime and
those have CLOCK_REALTIME as a macro */So the question becomes : how do we reliably determine that a given version of MinGW actually has It appears that there are portability problems with
This is a little puzzling to me, since it seems to say that the function is missing for MinGW, and it also doesn't work. Seems to me it should be one or the other. (Maybe they mean "missing on old MinGW and broken on newer MinGW"?) The implication is that we need to initialize The following patch, which simply says "don't use diff --git a/pdcurses/getch.c b/pdcurses/getch.c
index 78d0dbbb..a19ac8e5 100644
--- a/pdcurses/getch.c
+++ b/pdcurses/getch.c
@@ -419,9 +419,8 @@ use clock_gettime() or gettimeofday() when available. */
#endif
#if defined( _POSIX_C_SOURCE) && (_POSIX_C_SOURCE >= 199309L) \
- && (!defined( __MINGW32__) || defined( CLOCK_REALTIME))
- /* only newer MinGW environments have clock_gettime and
- those have CLOCK_REALTIME as a macro */
+ && !defined( __MINGW32__)
+ /* only newer MinGW environments have clock_gettime */
#define HAVE_CLOCK_GETTIME
#elif defined( _DEFAULT_SOURCE) || defined( _BSD_SOURCE) \
|| defined( __FreeBSD__) || defined( __MINGW32__)However (as it says just below that patch) "POSIX.1-2008 marks So ideally, we'd find out with which version of MinGW the |
That's... strange. can you run what we effectively do with this PR please?
? Note: we still may have to add |
|
Figured it out... So I am reasonably confident that your fix will work (works here, anyway). I am just sufficiently worried by these problems that I'd rather hold off on having it in a release version, though. The other possibility (maybe safer) is to say that we only use I'm going to go ahead with the release "as is" (basically just change version constants and, in |
|
Sounds good to me. |
note: while _POSIX_TIMERS may be "indirectly" defined by including time.h on some systems, they come from unistd.h.
tested to work on Debian and MSYS2 (gcc + clang)
fixes #345 (C part)
to be checked:
Is there an environment where this does not work (it seems that on non-released mingwrt we do have the feature with clock_gettime since Dec 2017 but no posix-feature-macros at all; not sure if this is something to care for)?
Should we apply something similar to other places? See https://linux.die.net/man/7/posixoptions.